Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

include: split public and private headers & prototype libsclang interface #703

Merged

Conversation

timblechmann
Copy link
Contributor

hi all,

i'd like to merge this patch. it does:

  • split the headers into public (in include/) and private (in lang/ or server/).
  • clean up the public interface for libsclang (only expose the SC_TerminalClient class)
  • remove some dead code

jakob, two points for you:

  • i know you have been working on a plugin interface for the language. that should be unrelated to the changes of the libsclang interface, as i've only dealt with the interface, how to use libsclang.
  • i'm not sure what's the best way to incorporate QtCollider::LangClient, but maybe we can simply provide a factory function which returns a TerminalClient *, which could be a QtCollider::LangClient instance.

thoughs?

Signed-off-by: Tim Blechmann tim@klingt.org

@jleben
Copy link
Member

jleben commented Jan 1, 2013

i know you have been working on a plugin interface for the language. that should be unrelated to the changes of the libsclang interface, as i've only dealt with the interface, how to use libsclang.

Not precisely. The plugin interface also modifies the headers affected by this patch, in order to extract a minimal needed subset of API. But that means only an inconvenience and is no big deal to solve, so never mind.

i'm not sure what's the best way to incorporate QtCollider::LangClient, but maybe we can simply provide a factory function which returns a TerminalClient *, which could be a QtCollider::LangClient instance

Sounds good!

@kaoskorobase
Copy link
Contributor

fwiw, LanguageClient was meant to be an abstract public interface and should be returned by the factory function, unless it's missing functionality?

@jleben
Copy link
Member

jleben commented Jan 2, 2013

fwiw, LanguageClient was meant to be an abstract public interface and should be returned by the factory function, unless it's missing functionality?

Both LanguageClient and TerminalClient are useful publicly: the latter has additional public interface to be used externally, and would have no sense as a virtual interface of LanguageClient.

@timblechmann
Copy link
Contributor Author

jakob, which parts of SC_TerminalClient do you want to expose publicly? i suppose run or quit would probably make sense? (at least run is used in cmdLineFuncs.cpp)

@timblechmann
Copy link
Contributor Author

also, stefan, why did you expose the symbols? afaict the methods using them could be be hidden in the implementation, so that PyrSymbol could be moved completely out of the public interface

@kaoskorobase
Copy link
Contributor

it was meant as an optimization for often sent symbols and i think i've used them in a GUI client once. i think it would be better to hide them and keep PyrSymbol out of the public interface as you say. thanks for this patch, it was long needed ;)

…face

Signed-off-by: Tim Blechmann <tim@klingt.org>
@timblechmann
Copy link
Contributor Author

looking at the code, i'd suggest to move SC_StringBuffer out of the public interface. one cannot use it without compiling the related source file, but it is trivial to use the LanguageClient without ...

@kaoskorobase
Copy link
Contributor

On 03/01/2013, at 10:23, Tim Blechmann notifications@github.com wrote:

looking at the code, i'd suggest to move SC_StringBuffer out of the public interface. one cannot use it without compiling the related source file, but it is trivial to use the LanguageClient without ...

ok, but you would need to link against libsclang anyway so no need to compile the module for users of the library, no? maybe an stl equivalent could be used instead? i wrote the string buffer only because james was reluctant to depend on the stl back then.

sk

@timblechmann
Copy link
Contributor Author

linking to the library is one thing, exporting the symbols of the class is another (atm SC_StringBuffer is not exported). but afaict it does not look too different from a std::string or std::vector

Signed-off-by: Tim Blechmann <tim@klingt.org>
Signed-off-by: Tim Blechmann <tim@klingt.org>
Signed-off-by: Tim Blechmann <tim@klingt.org>
cleans up the initialization code for primitives

Signed-off-by: Tim Blechmann <tim@klingt.org>
Signed-off-by: Tim Blechmann <tim@klingt.org>
@timblechmann
Copy link
Contributor Author

ok, after exposing run() i think we can even move the SC_TerminalClient out of the public interface. jakob, i've changed the initialization of qtcollider a bit (moving it to the factory). could you double-check if everything is correct?

Signed-off-by: Tim Blechmann <tim@klingt.org>
plugins only acquire the NRT lock, but should to that via the interface
table

Signed-off-by: Tim Blechmann <tim@klingt.org>
Signed-off-by: Tim Blechmann <tim@klingt.org>
timblechmann added a commit that referenced this pull request Jan 7, 2013
include: split public and private headers & prototype libsclang interface
@timblechmann timblechmann merged commit 64e78cf into supercollider:master Jan 7, 2013
@2mc 2mc mentioned this pull request Jan 8, 2013
timblechmann added a commit that referenced this pull request Jan 8, 2013
Merge pull request #703 broke 'no-ide compiles' due to the duplicate symbol 's_tick' introduced by commit 8807e9e. I guess, occurrence of 's_tick' in 'GUIPrimitives.m' is a left-over and should be removed there.
@timblechmann timblechmann deleted the topic/public_interface branch July 26, 2015 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants